Implemented Durable Graph Database Providers ( golem:graph WIT interfaces) #40
Conversation
|
#Maintainers this pr contains unit tests that are ignored when env variables are not set , this test are the MOTIVATION for our upcoming golem integeration tests ! we will refactor this soon . |
|
update : testing has been completed for integration , wasm component based testing is undergoing ! |
Screencast.from.2025-06-23.23-10-12.webmarangodb
|
Screencast.from.2025-06-24.16-16-07.webmJanusgraph
|
Screencast.from.2025-06-24.22-41-12.webmNeo4j
|
Tip : i also specified the env variables for local , in golem.yaml you have to just uncomment it which ever provider your running .also this all three are well tested under the real instances at each level , using ureq . |
| golem-rust = { workspace = true } | ||
| log = { workspace = true } | ||
| serde_json = { workspace = true } | ||
| mime = "0.3.17" |
There was a problem hiding this comment.
Please remove unused crates (in other Cargo.toml-s too)
| use std::env; | ||
|
|
||
| /// Retrieves a configuration value from an environment variable, checking the provider_config first. | ||
| pub fn with_config_key(config: &ConnectionConfig, key: &str) -> Option<String> { |
| tx_url: &str, | ||
| statements: Value, | ||
| ) -> Result<Value, GraphError> { | ||
| println!("[Neo4jApi] Cypher request: {statements}"); |
There was a problem hiding this comment.
Please use log functions instead of println-s and eprintln-s
| use crate::golem::graph::types::ElementId; | ||
|
|
||
| /// Helper functions for creating specific error types | ||
| pub fn unsupported_operation<T>(message: &str) -> Result<T, GraphError> { |
There was a problem hiding this comment.
It seems that these helpers are not used
| use std::collections::HashMap; | ||
|
|
||
| /// Database-agnostic error mapper that can be specialized by providers | ||
| pub struct ErrorMapper { |
There was a problem hiding this comment.
This is also seem to be not used at all.
| if response.status().is_success() { | ||
| Ok(response) | ||
| } else { | ||
| let status_code = response.status().as_u16(); |
There was a problem hiding this comment.
The error handling does not seems to match the Neo4j HTTP API error format, and does not extract or map the Neo4j specific error codes to the wit error model.
Also the error handling and mapping could be extracted, and would be nice to have at least a sample of the error payload in the generic case, if parsing as error failed (so one can see e.g. errors introduced by proxies)
| } | ||
|
|
||
| /// HTTP status code to GraphError mapping | ||
| pub fn map_http_status( |
There was a problem hiding this comment.
Are status codes are shared between different Graph APIs? Also, we should not rely on string searches for error mapping, we should rely on the specific error formats of the different providers, and those mappings should be part of the specific sub-crate.
| } | ||
|
|
||
| /// Map ArangoDB-specific error code to GraphError | ||
| pub fn from_arangodb_error_code(error_code: i64, message: &str) -> GraphError { |
There was a problem hiding this comment.
This mapping should not be part of the common graph crate
|
A generic request for the provider tests: these should be added (e.g. as scripts), and should be part of the CI flow |
/claim #22
/closes #22
note : the testing is undergoing with maintainer reviews , this is initial and core version of golem:graph with solid foundation , thank you !